-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Display figures on a new github pages site. #49
Conversation
This all looks good but I'm wondering how this actually works on the repository. We just went through a big cleaning process and we don't want to have to deal with that again, so I'm hyper vigilant about uploading things to the repository. It seems like this pushes the pdfs to the repository each time. I see this discussion about that, for example. That then points to this as a way to push but not keep the history. In short, I think whatever we do, we don't want the history of the pdfs pushed, just the most recent. |
b8539a1
to
c7946c4
Compare
c7946c4
to
84f3796
Compare
Thanks @etpeterson for the review, I used the deployment method you suggested and it worked fine, see: https://github.com/AhmedBasem20/TF2.4_IVIM-MRI_CodeCollection/actions/runs/8139256741 |
This run only ran 4 algorithms but I see on the hosted site that all of them ran. Why this discrepancy? I don't see anything in the code that should be changing that. I also see there are a few pushes that caused commits but I don't see any changes more recently. I'm not sure how this should act, but with the algorithm discrepancy I'm still hesitant to merge until it's all clear. |
My bad. I wanted to see the results quickly, so I removed most of the algorithms on this workflow. But the workflow of the committed code on this pull request works fine: https://github.com/AhmedBasem20/TF2.4_IVIM-MRI_CodeCollection/actions/runs/8139531990
Those pushes caused by the old action that deploys to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, just one small comment. It is a little opaque to me how the publishing works though. Have you found a good resource?
Perhaps another change would be however we should structure the website. I'm imagining a base page and we link to the results from there, or perhaps they're on the base page somewhere. Right now it's just the results page, correct? Maybe the comment in the related issue will get answered to help here.
@etpeterson the publishing action depends on the artifact we uploaded, I used this action as a reference. |
This looks good. I think there's an open question about where the results should land and if the index.html page should start with a little more on there, but that's for @petravanhoudt and @oliverchampion. |
Just following up @oliverchampion or @petravanhoudt if you have ideas on the structure of the page. |
superseded by #59 |
Describe the changes you have made in this PR
Displayed the generated plots from the analysis workflow to a new github-pages site.
Results
Example from my fork: https://ahmedbasem20.github.io/TF2.4_IVIM-MRI_CodeCollection/
@etpeterson To enable this on this repository we need the following steps:
gh-pages
.gh-pages
as a source branch.Link this PR to an issue [optional]
Fixes #46